Skip to content

Add df to the params of the T distributions - #440

Open
alejandroschuler wants to merge 1 commit into
masterfrom
fix/t-params-df
Open

alejandroschuler wants to merge 1 commit into
masterfrom
fix/t-params-df

Conversation

@alejandroschuler

Copy link
Copy Markdown
Collaborator

Found while assessing #168.

The params property of T, TFixedDf and TFixedDfFixedVar returned only loc and scale, although each of these distributions has a df. So scipy.stats.t(**dist.params) raised a TypeError, and any tool that reads params could not see the degrees of freedom. notebooks/example_t.ipynb read dists.df directly to work around this.

This PR adds df to params for all three classes. Cauchy and CauchyFixedVar inherit from TFixedDf and TFixedDfFixedVar, so they now report df equal to 1. The existing keys and their values do not change. The diff is three one-line edits in ngboost/distns/t.py, kept away from the __getattr__ methods that #433 edits. A trial merge of this branch with #433 has no conflicts.

Example

import numpy as np
import scipy.stats
from ngboost.distns import TFixedDf

dist = TFixedDf(np.array([[0.0, 1.0], [0.0, 0.0]]))  # loc, log(scale)
dist.params
scipy.stats.t(**dist.params).logpdf(np.array([0.5, 2.0]))
dist.params scipy.stats.t(**dist.params)
master {'loc': ..., 'scale': ...} TypeError: _parse_args() missing 1 required positional argument: 'df'
this PR {'loc': array([0., 1.]), 'scale': array([1., 1.]), 'df': array([3., 3.])} logpdf is [-1.161, -1.576], equal to dist.logpdf

Tests

  • test_t_family_params_match_scipy runs for T, TFixedDf, TFixedDfFixedVar, Cauchy and CauchyFixedVar. For each class it checks that params has the keys loc, scale and df, that loc and df keep their values, that df is 3 or 1 for the fixed-df classes, and that scipy.stats.t(**dist.params) gives the same logpdf and ppf as the distribution. All 5 cases fail on master.
  • Full suite: 139 passed, 59 skipped (134 passed, 59 skipped on master). The 5 new cases are the whole difference.
  • black, isort, flake8 and pylint are clean on the changed Python files, with the options from .pre-commit-config.yaml.

notebooks/example_t.ipynb has one code line changed, so that the density plot uses **dists[i].params. The stored outputs in the notebook are unchanged, and I did not run it again.

🤖 Generated with Claude Code

The params property of T, TFixedDf and TFixedDfFixedVar returned only loc
and scale, although each of these distributions has a df. So
scipy.stats.t(**dist.params) raised a TypeError, and any tool that reads
params could not see the degrees of freedom. The example notebook read
dists.df directly to work around this.

params now includes df for all three classes. Cauchy and CauchyFixedVar
inherit the change, with df equal to 1. The existing keys and their values
do not change. The example notebook now passes dists[i].params to
scipy.stats.t.pdf.

Found while assessing #168.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant